UI tests for Gatsbygram #4790
UI tests for Gatsbygram #4790
Conversation
ghost
assigned 
|
Deploy preview for using-drupal ready! Built with commit 6748a67 |
|
Deploy preview for gatsbygram ready! Built with commit 5804008766922406f0a3990821e7f3d18c507e4e |
|
Looks like we'll need a Gatsby plugin which uses babel-plugin-react-remove-properties @KyleAMathews - would you prefer this to live in this repo as an official plugin or keep it outside? |
|
Oh nice! Yeah, it'd be great to have some UI tests run alongside the other tests. This is something that we're sorely missing atm because we could accidentally break Gatsby's runtime (happened a few times...) and not see it until it's deployed. |
|
I don't think the test properties matter much so let's not worry about removing them for the production build. |
|
@KyleAMathews I've updated the Travis config to run UI tests as well and set Cypress to record video of tests that can viewed later on Cypress Dashboard (have added you to the gatsbyjs org there, check your email). For the video recording to work, we need an environment variable Value for this variable is available here - https://dashboard.cypress.io/#/projects/c9d3r3/settings |
| @@ -27,6 +27,23 @@ jobs: | |||
| script: | |||
| - yarn test | |||
|
|
|||
| - stage: gatsbygram ui tests | |||
| language: node_js | |||
There was a problem hiding this comment.
@tsriram Can you add the env var here (maybe you tried that already)? Travis might not pick it up until this branch is merged.
There was a problem hiding this comment.
@m-allanson Ah, didn't know that ENV variables are not accessible before merging. Nope, I didn't try adding the variable here. Adding it here will make it public right? I'm not sure if it's okay. From Cypress docs:
If you have a public project you should still keep your record key secret. If someone knows both your record key and your projectId, they could record test runs for your project - which would mix up all of your results!
Wonder if it's okay for us to make it public? If yes, we can just pass the key from the npm script itself.
There was a problem hiding this comment.
Ah, didn't know that ENV variables are not accessible before merging.
Oh hrmm, it seems I was wrong about that, Travis will use the yaml file from the current branch: https://docs.travis-ci.com/user/customizing-the-build/#Building-Specific-Branches
Yeah adding it here will make it public, but you can use the Travis CLI to encrypt env vars. See an example here.
Although pay attention to their note:
Encryption and decryption keys are tied to the repository. If you fork a project and add it to Travis CI, it will not have access to the encrypted variables.
Otherwise you may spend a lot of time (like I did 😅) wondering why your env var isn't working. It may be easier to merge this branch as-is, and then create smaller PRs straight from this repo to tinker with the travis.yml config.
Alternatively @KyleAMathews could add the env var via the Travis site (as you mentioned originally) which would certainly be quicker :), but I think it's nicer to keep the Travis config under version control where possible.
There was a problem hiding this comment.
Oh hrmm, it seems I was wrong about that, Travis will use the yaml file from the current branch
Probably not 😄 Looks like the environment variables defined in repository settings are not "automatically" available to forks: https://docs.travis-ci.com/user/environment-variables/#Defining-Variables-in-Repository-Settings. I think even if @KyleAMathews adds the env variable, it's not going to work. So, as you said it's probably better to merge this and have a branch from this repo itself to get this working.
|
@KyleAMathews I think this is ready to go in if all looks good to you. @tsriram will have a follow up PR to get the Travis config sorted. |
There was a problem hiding this comment.
Oh Travis is failing... Let's get that working again and then merge this in. @tsriram I've added a comment.
examples/gatsbygram/package.json
Outdated
| "deploy": "gatsby build --prefix-paths && gh-pages -d public" | ||
| "deploy": "gatsby build --prefix-paths && gh-pages -d public", | ||
| "cy:open": "cypress open", | ||
| "cy:run": "cypress run --record" |
There was a problem hiding this comment.
@tsriram If this --record key is removed, that should make the CI tests pass again?
Then --record can be added back in a follow-up PR, along with the env var.
There was a problem hiding this comment.
Yep, makes sense. Will remove --record for now 👍
tsriram
force-pushed
the
gatsbygram-cypress-test
branch
from
269cf93
to
5804008
Compare
|
@m-allanson Builds are passing now except the DCO check. I think I've signed the commit correctly (amended it), but not sure why it's failing. Am I missing something? Edit: I missed signing off all commits in the PR and for some reason |
Signed-off-by: Sriram Thiagarajan <sri.sjc@gmail.com>
Signed-off-by: Sriram Thiagarajan <sri.sjc@gmail.com>
Signed-off-by: Sriram Thiagarajan <sri.sjc@gmail.com>
Signed-off-by: Sriram Thiagarajan <sri.sjc@gmail.com>
Signed-off-by: Sriram Thiagarajan <sri.sjc@gmail.com>
Signed-off-by: Sriram Thiagarajan <sri.sjc@gmail.com>
Signed-off-by: Sriram Thiagarajan <sri.sjc@gmail.com>
* update all gatsby dependencies to latest tag * run cypress tests on travis, hopefully * well, try again to run ui tests on travis * add cypress videos to gitignore * uncomment original travis stages * update cypress project id Signed-off-by: Sriram Thiagarajan <sri.sjc@gmail.com>
Signed-off-by: Sriram Thiagarajan <sri.sjc@gmail.com>
tsriram
force-pushed
the
gatsbygram-cypress-test
branch
from
5804008
to
6748a67
Compare
|
Excellent work! Really excited to have UI tests running now 🎉 |
First things first, here's a video:
Taking #4361 forward, I've written UI tests of the example site Gatsbygram using Cypress. This is my first time writing tests using Cypress so it's quite possible that I'm not doing things in the best possible way 😄
Though I think there are enough tests to know whether Gatsbygram is working, this is still a WIP PR as we'd need to set up CI to run UI tests along with other tests - wonder if we need to run these tests for all PRs? Would love to get some feedback from people who've worked with Cypress.
TODO:
Remove data-test* attributes from production builds